refactor: move storage schema component into a separate file#2603
refactor: move storage schema component into a separate file#2603
Conversation
| static STORAGE_SCHEMA_LIBRARY: LazyLock<Library> = LazyLock::new(|| { | ||
| let bytes = include_bytes!(concat!( | ||
| env!("OUT_DIR"), | ||
| "/assets/account_components/metadata/schema_commitment.masl" | ||
| )); | ||
| Library::read_from_bytes(bytes).expect("Shipped Storage Schema library is well-formed") | ||
| }); |
There was a problem hiding this comment.
I think we should start moving these into their corresponding module, and eventually, try to get rid of the StandardAccountComponent (I may be forgetting something, but it is not clear to me why we need it).
| /// Name of the component is set to match the path of the corresponding module in the standards | ||
| /// library. | ||
| const NAME: &str = "miden::standards::metadata::storage_schema"; |
There was a problem hiding this comment.
I changed the name of the component to be the same as its module path in the standards library.
| static SCHEMA_COMMITMENT_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| { | ||
| StorageSlotName::new("miden::standards::metadata::storage_schema::commitment") | ||
| .expect("storage slot name should be valid") | ||
| }); |
There was a problem hiding this comment.
Also, changed the name of the storage slot to make it consistent with the name of the component.
| AccountComponent::new(STORAGE_SCHEMA_LIBRARY.clone(), storage, metadata) | ||
| .expect("AccountSchemaCommitment is a valid account component") |
There was a problem hiding this comment.
It seems like AccountComponent::new() does not verify consistency between storage and metadata. @igamigo - is this intentional? If not, let's create an issue and fix it.
There was a problem hiding this comment.
I don't think this was intentional. I'll create an issue to track this.
PhilippGackstatter
left a comment
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me!
I did not re-review most of the component, assuming it was moved.
|
I'll merge this as is. @igamigo - would still be good if you could take a look at this, especially at #2603 (comment) and #2603 (comment). |
Sorry, had a pending review from yesterday but wanted to review what was there and what wasn't in terms of validations for this comment |
3 participants
This PR moves the
AccountSchemaComponentinto a separate module. The main intent here is to make the diff in #2439 smaller, but it also fixes a couple of things in the component (see inline comments).